Skip to content

fix(fs): avoid directory fsync on Windows publication - #48

Merged
alkoleft merged 2 commits into
alkoleft:masterfrom
korolevpavel:fix/issue-42-windows-publication
Aug 2, 2026
Merged

fix(fs): avoid directory fsync on Windows publication#48
alkoleft merged 2 commits into
alkoleft:masterfrom
korolevpavel:fix/issue-42-windows-publication

Conversation

@korolevpavel

@korolevpavel korolevpavel commented Jul 26, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Closes #42.

Moves directory open/fsync fully behind cfg(unix), so non-Unix publication succeeds after rename without trying to open the parent directory as a regular file.

Verification:

  • cargo test --offline support::fs::tests (9 passed)
  • focused publication tests (3 passed)
  • cargo fmt --all -- --check
  • cargo check --offline
  • git diff --check

A cfg(windows) regression uses a guaranteed missing path and must be executed in Windows CI/a Windows host; Windows target stdlib is unavailable locally.

Summary by CodeRabbit

  • Исправления
    • Исправлена публикация каталогов на Windows: при отсутствии целевого каталога данные теперь корректно перемещаются в новое расположение.
    • Устранены проблемы, из-за которых опубликованные файлы могли быть недоступны после операции.
    • Улучшена кроссплатформенная обработка синхронизации каталогов без изменения поведения на Unix-системах.

@korolevpavel
fix(fs): skip directory fsync outside unix
3073212 
- avoid opening directory paths on Windows and other non-Unix targets
- cover the Windows no-op behavior with a missing-path regression test
@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 79e327f6-2c13-48bd-b804-df3f47380a63

📥 Commits

Reviewing files that changed from the base of the PR and between d612e2d and 5365888.

📒 Files selected for processing (1)
  • src/support/fs.rs

Walkthrough

Платформенная логика best_effort_fsync_dir больше не открывает каталоги в Windows. Добавлен Windows-тест, проверяющий успешную публикацию staging-каталога в новый target, удаление staging-каталога и доступность опубликованного файла.

Changes

Публикация каталогов в Windows

Layer / File(s) Summary
Платформенная fsync-логика и регрессионная проверка
src/support/fs.rs		 Открытие каталога и fsync выполняются только в Unix-ветке; Windows-ветка не использует File::open. Добавлены условный импорт и тест публикации staging-каталога в новый target.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

Я兔ик скачет: каталог открыт —
Но Windows больше двери не зовёт.
Staging прыгнул в target без тревог,
Файл сияет, чист staging-уголок.
Проверка машет лапкой: «Всё готово!»

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Заголовок кратко и точно описывает основной фикс: отключение fsync каталога на Windows при публикации.
Linked Issues check ✅ Passed Изменение устраняет открытие каталога как файла вне Unix и добавляет Windows-регрессию для публикации в новый каталог.
Out of Scope Changes check ✅ Passed Внесённые изменения ограничены исправлением fsync и связанным тестом, лишнего функционала нет.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@alkoleft
test(fs): cover Windows directory publication
5365888 
- publish a staged directory into a fresh target in the regression test
- document the fsync unsafe invariant
@zeegin ChatGPT Codex Connector

zeegin commented Jul 31, 2026

Copy link
Copy Markdown

Этот PR необходим для исправления IngvarConsulting/unica#264.

В поставляемом с Unica Windows runtime ошибка воспроизводится при make для EXTERNAL_DATA_PROCESSORS: платформа успешно формирует и проверяет EPF, staged-файл существует и валиден, но финальная публикация каталога завершается кодом 3 из-за Access is denied (os error 5) при открытии родительского каталога внутри best_effort_fsync_dir.

Изменение в этом PR устраняет именно корневую причину: на Windows каталог больше не открывается как обычный файл, при этом ошибки компиляции, проверки, записи и переименования по-прежнему не подавляются.

Для Unica это критичный блокер Windows-сценария сборки внешних обработок: пока PR не слит и не выпущен новый бинарный asset v8-runner, корректно собранные EPF ошибочно возвращаются пользователю как неуспешная сборка. Просьба при возможности приоритизировать merge.

@alkoleft
alkoleft merged commit ff02ede into alkoleft:master Aug 2, 2026
5 checks passed
@zeegin zeegin mentioned this pull request Aug 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(windows): staged directory publication fails after successful rename with AccessDenied

3 participants

@korolevpavel @zeegin @alkoleft